Skip to content

Conversation

@ouvreboite
Copy link
Contributor

@ouvreboite ouvreboite commented May 3, 2023

Resolves #2383

  • Add non-regression tests to ensure [DataMember(IsRequired)] do not bypass existing [JsonObject], [JsonProperty] or [JsonRequired] attributes.
  • Add tests for [DataMember(IsRequired)] and [DataMember(name)]
  • Update NewtonsoftDataContractResolver to directly leverage jsonProperty.Required, instead of using the IsRequiredSpecifed extension method

Havunen added a commit to Havunen/DotSwashbuckle2 that referenced this pull request Feb 12, 2024
Havunen added a commit to Havunen/DotSwashbuckle that referenced this pull request Feb 18, 2024
@martincostello
Copy link
Collaborator

Thanks for contributing - if you'd like to continue with this pull request, please rebase against the default branch to pick up our new CI. We'd also require tests for this scenario.

@ouvreboite
Copy link
Contributor Author

I'll rebase once #2799 is merged

@martincostello
Copy link
Collaborator

FYI #2799 was merged a month ago.

As DataMember implementation comes after the implementation for [JsonProperty] and [JsonObject], we want to be sure that the current behavior is not bypassed.
Add test cases for DataMember (IsRequired and Name).
It seems that NewtonSoft's ResolveContract correctly set the Required, using [DataMember], [JsonProperpty] and [JsonRequired] so we can directly use it instead of the IsRequiredSpecifed extension method.
Newtonsoft do all the work for us, handling [JsonProperty(Required = Required.Always)] or [JsonRequired] or [DataMember(IsRequired = true)] and filling the JsonProperty.Required field.
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.61%. Comparing base (8f9c079) to head (77994ee).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2644      +/-   ##
==========================================
+ Coverage   88.46%   90.61%   +2.14%     
==========================================
  Files          95       74      -21     
  Lines        3364     2962     -402     
  Branches      577      469     -108     
==========================================
- Hits         2976     2684     -292     
+ Misses        388      278     -110     
Flag Coverage Δ
Linux 90.61% <ø> (ø)
Windows ?
macOS 90.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@martincostello martincostello added this to the v6.7.1 milestone Aug 1, 2024
@martincostello martincostello merged commit 2c5ec58 into domaindrivendev:master Aug 1, 2024
@martincostello
Copy link
Collaborator

Thanks!

@martincostello
Copy link
Collaborator

Version 6.7.1 is now available from NuGet.org which includes this change.

This was referenced Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DataMember(IsRequired = true)] do not mark property as required

3 participants